[PATCH 08/24] auth: Rewrite ldap_escape() with a unit test
authorTimo Sirainen <timo.sirainen@open-xchange.com>
Mon, 23 Feb 2026 17:54:40 +0000 (19:54 +0200)
committerNoah Meyerhans <noahm@debian.org>
Tue, 31 Mar 2026 19:07:17 +0000 (15:07 -0400)
Gbp-Pq: Name CVE-2026-24031-27860-5.patch

src/auth/Makefile.am
src/auth/db-ldap.c
src/auth/db-ldap.h
src/auth/test-auth.h
src/auth/test-ldap.c [new file with mode: 0644]
src/auth/test-main.c

index 1d4b5d0b1e65882d3700c0485d1b7cfe64d1d300..d3745b8023f77116a6a45fa9e5b476944959b15a 100644 (file)
@@ -178,6 +178,7 @@ libauthdb_ldap_la_LDFLAGS = -module -avoid-version -shared
 libauthdb_ldap_la_LIBADD = $(LIBDOVECOT_LDAP) $(LDAP_LIBS)
 libauthdb_ldap_la_CPPFLAGS = $(AM_CPPFLAGS) -DPLUGIN_BUILD
 libauthdb_ldap_la_SOURCES = $(ldap_sources)
+test_auth_ldadd_plugins += libauthdb_ldap.la
 else
 auth_libs += $(LIBDOVECOT_LDAP)
 endif
@@ -225,6 +226,7 @@ test_auth_SOURCES = \
        test-auth-request-var-expand.c \
        test-auth-request-fields.c \
        test-username-filter.c \
+       test-ldap.c \
        test-lua.c \
        test-mock.c \
        test-main.c
index 9dcebedd57e8a90fe3b337a8808df07f6b124cf6..8eda9245654450ff67b61addb5203cc032d4686c 100644 (file)
@@ -13,6 +13,7 @@
 #include "str.h"
 #include "strescape.h"
 #include "time-util.h"
+#include "unichar.h"
 #include "env-util.h"
 #include "settings.h"
 #include "ssl-settings.h"
@@ -54,8 +55,6 @@
 #define DB_LDAP_REQUEST_MAX_ATTEMPT_COUNT 3
 #define DB_LDAP_ATTR_DN "~dn"
 
-static const char *LDAP_ESCAPE_CHARS = "*,\\#+<>;\"()= ";
-
 struct db_ldap_result {
        int refcount;
        LDAPMessage *msg;
@@ -1139,26 +1138,80 @@ void db_ldap_get_attribute_names(pool_t pool,
                *sensitive_r = array_front(&sensitive_attr_names);
 }
 
-#define IS_LDAP_ESCAPED_CHAR(c) \
-       ((((unsigned char)(c)) & 0x80) != 0 || strchr(LDAP_ESCAPE_CHARS, (c)) != NULL)
-
-const char *ldap_escape(const char *str,
-                       void *context ATTR_UNUSED)
+const char *ldap_escape(const char *input, void *context ATTR_UNUSED)
 {
-       string_t *ret = NULL;
+       /* This function escapes both LDAP filters and LDAP DNs. This works,
+          because both allow using the method of escaping any characters.
+          However, this isn't really recommended, since apparently some LDAP
+          servers and perhaps other tools don't handle unnecessary escaping
+          correctly.
+
+          Another issue is what to do with invalid UTF-8. LDAP filter RFC
+          suggests just escaping invalid UTF-8. LDAP DN RFC doesn't at least
+          explicitly say that. It instead seems to imply that the string is
+          just expected to be valid UTF-8. Or perhaps to use #hex-string
+          encoding, which isn't compatible with LDAP filters. Just to be safe,
+          we'll replace invalid UTF-8 input with the UTF-8 replacement
+          character.
+
+          So this function isn't perhaps the most standards compliant one, but
+          we don't really care, since this escaping is mainly intended to
+          prevent untrusted username input from breaking out of the filter or
+          DN. Valid usernames are unlikely to require escaping or to be
+          invalid UTF-8. */
+
+       /* Convert invalid UTF-8 characters to replacement characters. */
+       size_t input_len = strlen(input);
+       string_t *str = t_str_new(64);
+       if (!uni_utf8_get_valid_data((const unsigned char *)input,
+                                    input_len, str)) {
+               /* Input was not valid UTF-8 */
+               input = t_strdup(str_c(str));
+               input_len = str_len(str);
+               str_truncate(str, 0);
+       }
 
-       for (const char *p = str; *p != '\0'; p++) {
-               if (IS_LDAP_ESCAPED_CHAR(*p)) {
-                       if (ret == NULL) {
-                               ret = t_str_new((size_t) (p - str) + 64);
-                               str_append_data(ret, str, (size_t) (p - str));
-                       }
-                       str_printfa(ret, "\\%02X", (unsigned char)*p);
-               } else if (ret != NULL)
-                       str_append_c(ret, *p);
+       const char *escape_chars =
+               /* control characters */
+               "\x01\x02\x03\x04\x05\x06\x07\x08"
+               "\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10"
+               "\x11\x12\x03\x14\x15\x16\x17\x18"
+               "\x19\x1a\x1b\x1c\x1d\x1e\x1f"
+               /* filter + DN */
+               "\\"
+               /* filter only */
+               "*()"
+               /* DN only (not including leading and trailing chars) */
+               ",+<>;\"=";
+       size_t pos;
+
+       /* check the leading character separately (required by DN) */
+       if (input[0] == '#' || input[0] == ' ')
+               pos = 0;
+       else {
+               pos = strcspn(input, escape_chars);
+               if (pos == input_len) {
+                       /* the last trailing space is escaped
+                          (required by DN) */
+                       if (pos > 0 && input[pos - 1] == ' ')
+                               pos--;
+                       else
+                               return input;
+               }
        }
 
-       return ret == NULL ? str : str_c(ret);
+       do {
+               str_append_data(str, input, pos);
+               str_printfa(str, "\\%02x", (unsigned char)input[pos]);
+               input += pos + 1;
+               input_len -= pos + 1;
+               pos = strcspn(input, escape_chars);
+               /* the last trailing space is escaped (required by DN) */
+               if (pos == input_len && pos > 0 && input[pos - 1] == ' ')
+                       pos--;
+       } while (pos < input_len);
+       str_append_data(str, input, pos);
+       return str_c(str);
 }
 
 static bool
index 7b2f60f65ff980826173ff9fa98498f26f8c07a0..a6aac02a72b6ba27cd67e6eec678becf522ad563 100644 (file)
@@ -169,6 +169,7 @@ void db_ldap_connect_delayed(struct ldap_connection *conn);
 
 void db_ldap_enable_input(struct ldap_connection *conn, bool enable);
 
+const char *ldap_dn_escape(const char *str, void *context);
 const char *ldap_escape(const char *str, void *context);
 const char *ldap_get_error(struct ldap_connection *conn);
 
index 517347a7c729a6f78bd789f99a8b975597a9c5c8..16e581dc4db8ed45bfb35e89473498b1e6b5b57e 100644 (file)
@@ -14,6 +14,7 @@ void test_auth_request_var_expand(void);
 void test_auth_request_fields(void);
 void test_db_dict_parse_cache_key(void);
 void test_username_filter(void);
+void test_db_ldap(void);
 void test_db_lua(void);
 struct auth_passdb *passdb_mock(void);
 void passdb_mock_mod_init(void);
diff --git a/src/auth/test-ldap.c b/src/auth/test-ldap.c
new file mode 100644 (file)
index 0000000..cd9904a
--- /dev/null
@@ -0,0 +1,39 @@
+/* Copyright (c) 2026 Dovecot authors, see the included COPYING file */
+
+#include "test-auth.h"
+
+#ifdef HAVE_LDAP
+#include "db-ldap.h"
+
+static void test_ldap_escape(void)
+{
+       struct {
+               const char *input;
+               const char *output;
+       } tests[] = {
+               { "", "" },
+               { " ", "\\20" },
+               { "  ", "\\20\\20" },
+               { "foo ", "foo\\20" },
+               { ",", "\\2c" },
+               { "s p a c e", "s p a c e" },
+               { "# start-end#", "\\23 start-end#" },
+               { "  start-end2  ", "\\20 start-end2 \\20" },
+               { "middle:,+\"\\<>;=", "middle:\\2c\\2b\\22\\5c\\3c\\3e\\3b\\3d" },
+               { "valid-utf8:\xc3\xb1", "valid-utf8:\xc3\xb1" },
+               { "Bad \xFF Characters", "Bad \xEF\xBF\xBD Characters" },
+       };
+       test_begin("ldap_escape()");
+       for (unsigned int i = 0; i < N_ELEMENTS(tests); i++) {
+               test_assert_strcmp_idx(ldap_escape(tests[i].input, NULL),
+                                      tests[i].output, i);
+       }
+       test_end();
+}
+
+void test_db_ldap(void)
+{
+       test_ldap_escape();
+}
+
+#endif
index cb05fda0571bffbcc2a31c03e49905363ef2cd5c..64467c43b5f55a42cd3d9b13045f21a7acb0e28b 100644 (file)
@@ -24,6 +24,9 @@ int main(int argc, char *argv[])
                TEST_NAMED(test_username_filter)
 #if defined(HAVE_LUA)
                TEST_NAMED(test_db_lua)
+#endif
+#if defined(HAVE_LDAP)
+               TEST_NAMED(test_db_ldap)
 #endif
                { NULL, NULL }
        };